Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only change mine data if using new allow_tgt feature #56172

Merged
merged 3 commits into from
Mar 10, 2020

Conversation

Ch3LL
Copy link
Contributor

@Ch3LL Ch3LL commented Feb 14, 2020

What does this PR do?

With the new allow_tgt feature in the mine.get the data structure was changed. This PR ensures we only send this new data structure if we are using the new allow_tgt feature.

Fixes the issue in #56118

The initial issue made it so a master on <3000 would not work with a >=3000 minion with mine data. With this PR we only change the data structure if we are using the new feature, so we assume if they are using the new feature they want to use 3000. Also documented that both the master and minion need to be on atleast versions 3000 if using this feature.

This PR also fixes another issue I found. If you set a minion in allow_tgt that is a minion that does not actually exist it will return the data regardless. Without this PR given the following config:

mine_functions:
  cpu:
    - mine_function: grains.get
    - cpuarch
    - allow_tgt: doesnotexist

As you can see above we have defined a minion that does not exist in allow_tgt, but I can still gather the data for any minion:

[root@localhost ~]# salt-call mine.get \* cpu
local:
    ----------
    master_min:
        x86_64
    thecakeisalie:
        x86_64

With this patch, we check to see if the minion even exists, if it does not we do not allow the minion to view the data:

[root@localhost ~]# salt-call mine.get \* cpu
local:
    ----------

What issues does this PR fix or reference?

Fixes #56118

Previous Behavior

Data returned on a master (<3000) and minion (>=3000), even if not using the new allow_tgt feature:

    minion_name:
        ----------
        __data__:
            - 10.1.2.3
        __saltmine_acl__:
            1

New Behavior

Data returned on a master (<3000) and minion (>=3000), even if not using the new allow_tgt feature:

    minion_name:
        - 10.1.2.3

To note if one is using the allow_tgt feature and they have a master <3000 they will see the wrong data (ex, __data__), this feature requires both master and minion are upgraded.

Tests written?

Yes

Commits signed with GPG?

Yes

@@ -213,9 +216,13 @@ def send(name, *args, **kwargs):
:param str mine_function: The name of the execution_module.function to run
and whose value will be stored in the salt mine. Defaults to ``name``.
:param str allow_tgt: Targeting specification for ACL. Specifies which minions
are allowed to access this function.
are allowed to access this function. Please note both your master and
minion need to be on atleast version 3000 for this to work properly.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

be on, at least, version?

:param str allow_tgt_type: Type of the targeting specification. This value will
be ignored if ``allow_tgt`` is not specified.
be ignored if ``allow_tgt`` is not specified. Please note both your
master and minion need to be on atleast version 3000 for this to work
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

@Ch3LL Ch3LL added v3000.1 vulnerable version Merge Ready labels Mar 4, 2020
@dwoz dwoz merged commit fb5252f into saltstack:master Mar 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v3000.1 vulnerable version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mine.get returns different dictionary format in version 3000
4 participants